feat(raycicmd): integrate matrix expansion into pipeline conversion#379
feat(raycicmd): integrate matrix expansion into pipeline conversion#379andrew-anyscale wants to merge 1 commit intoandrew/revup/main/matrix-expandfrom
Conversation
|
Summary of ChangesHello @andrew-anyscale, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the CI/CD pipeline's flexibility and efficiency by implementing dynamic matrix expansion and selective dependency management. It allows for the generation of specific pipeline steps based on matrix configurations and enables subsequent steps to depend only on relevant matrix variants. This improvement streamlines the build process by preventing the execution of redundant steps and provides finer control over pipeline dependencies. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
7466596 to
853ec65
Compare
7a80735 to
5b00dd8
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces a powerful matrix expansion and filtering feature for raycicmd. The implementation is well-structured, separating the matrix expansion logic into a new matrix_expand.go file and integrating it into the existing converter.go.
The changes include:
- Pre-processing of pipeline steps to expand matrix definitions into individual steps.
- Generation of a meta
waitstep to allow dependencies on an entire matrix. - Automatic generation of tags for each matrix instance to enable fine-grained filtering.
- Expansion of
depends_onselectors to target specific matrix instances.
The new functionality is thoroughly tested, covering expansion, dependency selection, tag filtering, and error conditions like missing label placeholders. A new Buildkite pipeline configuration is also added for integration testing.
I have one suggestion regarding the deep copy mechanism to make it more robust against future changes. Overall, this is a great addition that significantly enhances pipeline configuration capabilities.
5b00dd8 to
914104f
Compare
7450d3e to
b7b5455
Compare
914104f to
7b592fa
Compare
b7b5455 to
b488745
Compare
c5e6aa8 to
feac46f
Compare
4c8d9d2 to
83f7a55
Compare
b488745 to
bc2e07e
Compare
83f7a55 to
2d792f3
Compare
bc2e07e to
570e6e7
Compare
2d792f3 to
4214f2b
Compare
570e6e7 to
268d356
Compare
Code Review 👍 Approved with suggestions 1 resolved / 4 findingsSolid implementation of matrix expansion and filtering. Previous edge case findings regarding unchecked type assertions and silent failures remain unaddressed in this revision. 💡 Edge Case: Unchecked type assertion may panic on malformed stepAt line 128, Impact: Pipeline conversion could crash on edge case inputs. Suggested fix: Use a type assertion with ok check: expandedStepRaw := inst.substituteValues(step)
expandedStep, ok := expandedStepRaw.(map[string]any)
if !ok {
return nil, nil, fmt.Errorf("matrix step %q: substitution returned unexpected type", baseKey)
}💡 Edge Case: Silent failure when matrix selector matches no instances📄 raycicmd/converter.go:178-185 When Consider adding validation to warn or error when a matrix selector matches zero instances: matches, err := expandMatrixSelector(sel, ctx.registry, ctx.expandedKeys)
if err != nil {
return nil, err
}
if len(matches) == 0 {
return nil, fmt.Errorf("matrix selector for key %q matched no instances", sel.Key)
}
result = append(result, matches...)This would catch configuration errors early rather than silently dropping dependencies. 💡 Edge Case: Missing validation for empty depends_on after expansionIn If a user specifies: depends_on:
- key: build-step
matrix:
python: "3.99" # non-existent variantThe dependency would silently resolve to nothing, which could lead to steps running earlier than intended or missing required dependencies. Suggestion: Consider either returning an error when ✅ 1 resolved✅ Edge Case: Meta wait-step lacks tags, preventing tag-based filtering
OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
4214f2b to
0be5ac9
Compare
24b799a to
cbb9612
Compare
ff09e88 to
a9a0117
Compare
cbb9612 to
4ce2802
Compare
Wire matrix expansion into the step conversion flow. Matrix steps are expanded inline during conversion, with a meta wait-step created to preserve the original key for backwards-compatible depends_on references. ## Auto-Generated Tags Each expanded step receives auto-generated tags based on matrix values: python-3.11, cuda-12.1.1 These tags integrate with rayci's tag filtering system, allowing selective runs like "only run python-3.11 tests" via tag rules. ## Meta-Step Pattern The meta wait-step preserves backwards compatibility: - Key: original step key (e.g., "ray-build") - Type: wait step - depends_on: all expanded step keys This allows existing `depends_on: ray-build` references to wait for ALL matrix instances without modification. ## Filtering Behavior When tag filtering is applied: - Only matching expanded steps are included - If a step depends on the meta-step, ALL instances are pulled in - If a step uses selector syntax, only matching instances are included - If all instances are filtered out, the meta-step is also excluded Topic: matrix-converter Relative: matrix-expand Signed-off-by: andrew <andrew@anyscale.com>
a9a0117 to
b8182dd
Compare

Wire matrix expansion into the step conversion flow. Matrix steps are expanded inline during conversion, with a meta wait-step created to preserve the original key for backwards-compatible depends_on references.
Auto-Generated Tags
Each expanded step receives auto-generated tags based on matrix values:
python-3.11, cuda-12.1.1
These tags integrate with rayci's tag filtering system, allowing selective
runs like "only run python-3.11 tests" via tag rules.
Meta-Step Pattern
The meta wait-step preserves backwards compatibility:
This allows existing
depends_on: ray-buildreferences to wait for ALLmatrix instances without modification.
Filtering Behavior
When tag filtering is applied:
Topic: matrix-converter
Relative: matrix-expand
Signed-off-by: andrew andrew@anyscale.com